Skip to content

fix(layers): show warning if !Sub function is used in Layers section of template.yaml (#948)#949

Closed
goldeneggg wants to merge 2 commits intoaws:developfrom
goldeneggg:improve-message-in-local-lambda-layer
Closed

fix(layers): show warning if !Sub function is used in Layers section of template.yaml (#948)#949
goldeneggg wants to merge 2 commits intoaws:developfrom
goldeneggg:improve-message-in-local-lambda-layer

Conversation

@goldeneggg
Copy link
Contributor

@goldeneggg goldeneggg commented Jan 19, 2019

#948

Description of changes:

If !Sub function is used in Layers section of template.yaml and run sam local invoke and get an error.

  • got: Error reason is not clear
  • expected: We want to know more information by log
Resources:
  HelloWorldFunction:
    Type: AWS::Serverless::Function
    Properties:
      CodeUri: hello_world/
      Handler: app.lambdaHandler
      Runtime: nodejs8.10
      Events:
        HelloWorld:
          Type: Api
          Properties:
            Path: /hello
            Method: post
      Layers:
        - !Sub arn:aws:lambda:${AWS::Region}:123456789012:layer:MyLayer:1
$ sam local invoke -e event.json HelloWorldFunction

# This warning log is added by this pull request
2019-01-20 00:56:43 '!Sub' keyword in Layer configuration does not work locally. Please replace all parameter strings(like '${AWS::Region}') with the original value(like 'ap-northeast-1') and remove '!Sub' keyword.

Unable to import module 'app': Error

Checklist:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mnapoli
Copy link

mnapoli commented Feb 8, 2019

Stumbled on that as well, until such feature is supported it would be awesome to have a clear warning 👍

@sriram-mv sriram-mv requested a review from jfuss February 19, 2019 20:23
@jfuss
Copy link
Contributor

jfuss commented Feb 27, 2019

While this impacts Layers (probably disproportionally), I don't believe this is the correct place for adding this detail. SAM CLI (as a whole) does not have support for Sub (and many other intrinsics). Due to this, we shouldn't be trying to read these intrinsics within the higher provider classes. This means, for ever resource and property, we need to add this kind of logging. While it is contextually, it creates more work later on. Instead, I would be ok with adding this logging more broadly to communicate the known Intrinsic gaps. Maybe this is a good place?

@jfuss
Copy link
Contributor

jfuss commented Jul 22, 2019

@goldeneggg and @mnapoli We are addressing intrinsics in #1261. Once that is merged, integrated, and released, there shouldn't be any more intrinsic confusion on support. I am going to close this in favor of the full solution we are currently going through.

@jfuss jfuss closed this Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jfuss jfuss Awaiting requested review from jfuss

Assignees

@jfuss jfuss

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@goldeneggg @mnapoli @jfuss